Add app_spaces as a DABs resource type (direct mode only)#4982
Add app_spaces as a DABs resource type (direct mode only)#4982bernardo-rodriguez wants to merge 12 commits intodatabricks:mainfrom
Conversation
andrewnester
left a comment
There was a problem hiding this comment.
Could you please add acceptance test for the new resource, see for example
https://github.com/databricks/cli/tree/main/acceptance/bundle/resources/apps/update
| _, err := r.client.Apps.UpdateSpace(ctx, apps.UpdateSpaceRequest{ | ||
| Name: id, | ||
| Space: *config, | ||
| UpdateMask: fieldmask.FieldMask{Paths: []string{"description", "resources", "user_api_scopes", "usage_policy_id"}}, |
There was a problem hiding this comment.
Static list is fine, but I'm curious, is * not working for this endpoint?
There was a problem hiding this comment.
no, the server explicitly allowlists fields, using * fails with a server side validation error
| @@ -0,0 +1,17 @@ | |||
| bundle: | |||
There was a problem hiding this comment.
Please look into adding invariant tests as well - you need one config in acceptance/bundle/invariant/configs.
Does this resource support permissions?
There was a problem hiding this comment.
Yes, thanks for calling this out!
App Spaces support 3 different permissions levels (CAN_USE, CAN_CREATE, CAN_MANAGE) via /api/2.0/permissions/app-spaces/{space-name}. I added a new AppSpacePermission type to handle this.
Also, added that invariant config.
janniklasrose
left a comment
There was a problem hiding this comment.
Please address:
- test failures
- auto-gen relevant files
- add additional acceptance tests (definitely invariant, drift, also bind/unbind if relevant)
- opt out of terraform in
bundle/config/mutator/validate_direct_only_resources.go - add entry to NEXT_CHANGELOG.md
| title "Update name and re-deploy" | ||
| trace update_file.py databricks.yml myspacename mynewspacename | ||
| trace $CLI bundle plan | ||
| trace $CLI bundle summary | ||
| trace $CLI bundle deploy | ||
| trace print_requests >> out.requests.$DATABRICKS_BUNDLE_ENGINE.json |
There was a problem hiding this comment.
nit: same as recreate
There was a problem hiding this comment.
true. leaving for now as they have a different event sequence, but I can remove the rename state from the update test if you prefer a cleaner separation
| >>> [CLI] bundle plan | ||
| recreate app_spaces.mykey | ||
|
|
||
| Plan: 1 to add, 0 to change, 1 to delete, 0 unchanged | ||
|
|
||
| >>> [CLI] bundle deploy | ||
| Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... | ||
| Deploying resources... | ||
| Updating deployment state... | ||
| Deployment complete! |
There was a problem hiding this comment.
just confirming that recreating app_spaces is safe (no data loss)
There was a problem hiding this comment.
Yea that is fine, they are just metadata and store no customer data
There was a problem hiding this comment.
What happens if you delete an app space that has >0 apps attached?
There was a problem hiding this comment.
@pietern the server blocks deletion with 400 BAD_REQUEST: Cannot delete space <name> because it contains N app
6991ad3 to
81ced7f
Compare
| func (r *ResourceAppSpace) DoCreate(ctx context.Context, config *apps.Space) (string, *apps.Space, error) { | ||
| // Kick off the create request. Wait for the space to become active in | ||
| // WaitAfterCreate so that parallel creates are not blocked here. | ||
| _, err := r.client.Apps.CreateSpace(ctx, apps.CreateSpaceRequest{ |
There was a problem hiding this comment.
Is this not modeled after a long running operation?
There was a problem hiding this comment.
yes, I updated to match other examples of LRO, by using the waiter returned by the Create and Update method.
| >>> [CLI] bundle plan | ||
| recreate app_spaces.mykey | ||
|
|
||
| Plan: 1 to add, 0 to change, 1 to delete, 0 unchanged | ||
|
|
||
| >>> [CLI] bundle deploy | ||
| Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... | ||
| Deploying resources... | ||
| Updating deployment state... | ||
| Deployment complete! |
There was a problem hiding this comment.
What happens if you delete an app space that has >0 apps attached?
Adds support for declaring Databricks App Spaces in bundle YAML via the `app_spaces` resource type. Spaces are containers for apps that provide shared resources, OAuth scopes, and a service principal. Implements direct mode CRUD with async operation waiting, a merge mutator for the space resources array, run_as validation, and test server handlers for the fake workspace. Co-authored-by: Isaac
The Spaces API rejects wildcard update masks. Use the explicit list of updatable fields: description, resources, user_api_scopes, usage_policy_id. Co-authored-by: Isaac
- Add nolint comment for Id field collision between BaseResource and apps.Space (same pattern as App resource) - Regenerate annotations.yml and jsonschema.json via make schema - Add acceptance tests: basic (with nested resources), recreate (name immutability), update (create/update/rename/destroy lifecycle) Co-authored-by: Isaac
Previous golden files were generated with Python 3.6 which doesn't support f-string self-documenting expressions used in update_file.py, causing bogus SyntaxError output that would fail in CI. Co-authored-by: Isaac
Previously DoCreate and DoUpdate blocked on waiter.Wait, preventing the direct engine from parallelizing operations across resources. Move the wait into dedicated WaitAfterCreate/WaitAfterUpdate methods that poll GetSpace until the space reaches SPACE_ACTIVE state. Addresses review feedback from @andrewnester. Co-authored-by: Isaac
- Scope govet lint directive to a specific linter (Jannik)
- Remove fabricated InitializeURL; spaces have no stable UI URL yet (Jannik)
- Opt out of terraform via ValidateDirectOnlyResources (Jannik)
- Replace panic in merge_app_spaces with user-facing diagnostic (Denik)
- Add permissions support for app_spaces using the generic
/api/2.0/permissions/app-spaces/{name} endpoint
- Use print_requests.py helper in acceptance scripts (Denik)
- Add invariant, drift (via invariant no_drift), and bind/unbind
acceptance tests (Jannik)
- Exclude app_space from migrate invariant test since the resource is
direct-only (same pattern as catalog, external_location)
- Regenerate annotations, jsonschema, enum_fields, required_fields
- Fix TestConvertLifecycleForAllResources and state_load tests to
account for the new resource
- Add NEXT_CHANGELOG.md entry
Note: resources.generated.yml needs make generate-direct to populate
output-only fields (create_time, service_principal_*, status, etc.)
from the OpenAPI spec. Until then, invariant no_drift for app_space
will fail locally; CI pipelines with VPN access will keep it in sync.
Co-authored-by: Isaac
Co-authored-by: Isaac
- NEXT_CHANGELOG.md: remove bullets for databricks#4672 and databricks#4941 that were accidentally re-added during a stash/pop across a v0.297 release cut - acceptance/bundle/invariant/{continue_293,migrate,no_drift}/out.test.toml: restore job_with_depends_on.yml.tmpl that got dropped during a merge-conflict resolution on an earlier rebase Co-authored-by: Isaac
…oUpdate Co-authored-by: Isaac
9823c27 to
e8ce30b
Compare
Co-authored-by: Isaac
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Changes
Adds support for declaring Databricks App Spaces in bundle YAML via the
app_spacesresource type. Spaces are containers for apps that provide shared resources, OAuth scopes, and a service principal.Implements direct mode CRUD with async operation waiting, a merge mutator for the space resources array, run_as validation, and test server handlers for the fake workspace.
Why
App spaces are are in private preview, we want to add support for the developer ecosystem.
Tests
Tested with local CLI build.
Test databricks.yml file:
Commands:
New app space:
New App: